apps/mail/components/mail/mail-iframe.tsx (1)

101-106: Consider conditionally rendering the Trust Sender button.

The "Trust Sender" button is currently always shown when images are hidden, even if the sender is already trusted. Consider conditionally rendering this button only when the sender is not already trusted.

- <button
-   onClick={() => void onTrustSender(senderEmail)}
-   className="ml-2 cursor-pointer underline"
- >
-   {t('common.actions.trustSender')}
- </button>
+ {!isTrustedSender && (
+   <button
+     onClick={() => void onTrustSender(senderEmail)}
+     className="ml-2 cursor-pointer underline"
+   >
+     {t('common.actions.trustSender')}
+   </button>
+ )}

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

apps/mail/components/mail/mail-iframe.tsx (2)

22-42: Prevent duplicate entries in trustedSenders array

The current implementation doesn't check if the sender is already in the trusted list before adding it, which could lead to duplicate entries.

const onTrustSender = useCallback(async (senderEmail: string) => {
  setImagesEnabled(true);

  const existingSettings = settings ?? {
    ...defaultUserSettings,
    timezone: getBrowserTimezone(),
  };

  const { success } = await saveUserSettings({
    ...existingSettings,
    trustedSenders: settings?.trustedSenders
-      ? settings.trustedSenders.concat(senderEmail)
+      ? settings.trustedSenders.includes(senderEmail)
+        ? settings.trustedSenders
+        : settings.trustedSenders.concat(senderEmail)
      : [senderEmail],
  });

  if (!success) {
    toast.error('Failed to trust sender');
  } else {
    mutate();
  }
}, [settings, mutate]);

---

12-17: 🛠️ Refactor suggestion

Consider validating senderEmail for edge cases

Since senderEmail is now a required prop and used in critical checks, you should add validation to handle edge cases like empty strings or undefined values.

export function MailIframe({ html, senderEmail }: { html: string; senderEmail: string }) {
+  const validSenderEmail = senderEmail?.trim() || '';
  const { settings, mutate } = useSettings();
-  const isTrustedSender = settings?.trustedSenders?.includes(senderEmail);
+  const isTrustedSender = settings?.trustedSenders?.includes(validSenderEmail);
  const [imagesEnabled, setImagesEnabled] = useState(
    isTrustedSender || settings?.externalImages || false,
  );

apps/mail/components/mail/mail-iframe.tsx (3)

101-106: Hide or disable the trust button for already trusted senders

The "Trust Sender" button is always visible, even if the sender is already in the trusted senders list. This could confuse users who might try to trust a sender multiple times.

{!imagesEnabled && !settings?.externalImages && (
  <div className="flex items-center justify-start bg-amber-500 p-2 text-sm text-amber-900">
    <p>{t('common.actions.hiddenImagesWarning')}</p>
    <button
      onClick={() => setImagesEnabled(!imagesEnabled)}
      className="ml-2 cursor-pointer underline"
    >
      {imagesEnabled ? t('common.actions.disableImages') : t('common.actions.showImages')}
    </button>
+    {!isTrustedSender && (
      <button
        onClick={() => onTrustSender(senderEmail)}
        className="ml-2 cursor-pointer underline"
      >
        {t('common.actions.trustSender')}
      </button>
+    )}
  </div>
)}

---

37-41: Improve error handling for saveUserSettings

The current error handling only shows a toast message. Consider adding more robust error handling to provide a better user experience, especially for network failures.

if (!success) {
  toast.error('Failed to trust sender');
+  // Revert UI state since operation failed
+  setImagesEnabled(settings?.externalImages || false);
} else {
  mutate();
+  toast.success(t('common.actions.senderTrusted'));
}

---

30-35: Add a helper function for managing trusted senders

The logic for updating the trusted senders array could be extracted into a reusable helper function for better maintainability and to ensure consistent behavior across the application.

+// Add this helper function at the top of the file or in a separate utility file
+const addTrustedSender = (currentList: string[] = [], newSender: string): string[] => {
+  if (!newSender || currentList.includes(newSender)) {
+    return currentList;
+  }
+  return [...currentList, newSender];
+};

// Then in the component
const { success } = await saveUserSettings({
  ...existingSettings,
-  trustedSenders: settings?.trustedSenders
-    ? settings.trustedSenders.concat(senderEmail)
-    : [senderEmail],
+  trustedSenders: addTrustedSender(settings?.trustedSenders, senderEmail),
});

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro